fix(core): a lone failing cached call no longer kills the process (#670) - #676

Fixes #670. A cached stitch whose vendor fails, with no concurrent caller, emitted an unhandled promise rejection — which under Node's default --unhandled-rejections=throw terminates the process. The caller had already handled the failure: .safe() returned an honest ok: false, and the program died anyway.

Reproduced first, on unpatched main

The issue's 14-line reproduction, run against main's source:

caller handled it: ok = false
packages/core/src/engine.ts:1693
            claim.fail(new Error('cache: leader run failed'));
                       ^

Error: cache: leader run failed
    at runCached (engine.ts:1693:24)
    at async execute (engine.ts:1769:9)
    at async wrapped (stitch.ts:829:26)
    at async drain (stitch.ts:526:22)
    at async consumeSafe (stitch.ts:804:21)

Node.js v24.18.1
EXIT CODE: 1

STILL ALIVE — the line after the handled call — never printed. After:

caller handled it: ok = false
STILL ALIVE
EXIT CODE: 0

The issue's severity table reproduces exactly, and every row is now zero:

That second row is the whole reason this survived: a test exercises coalescing with a concurrent burst, and a follower's await catches the rejection by accident. A webhook backlog or a retry drain arrives staggered, where every call is its own leader.

The fix, and why this shape

One statement, in InflightCoalescer.join, attached to the shared promise where it is created:

const promise = new Promise<T>((res, rej) => { resolve = res; reject = rej; });
promise.catch(() => { /* an audience of nobody is not an error */ });

The reasoning, since "add a .catch" is exactly the move that deserves scrutiny:

It swallows nothing a follower needs. .catch() returns a derived promise, which is discarded. entry.promise — the object a follower awaits — is untouched. A follower's await claim.promise still rejects, same tick, same error identity. Verified rather than asserted: with three concurrent callers against a failing vendor, all three still receive their own honest HTTP 503 (never the cache: leader run failed sentinel) and the adapter is still called three times, before and after. What is retired is the coalescer's own liability, not the channel.

That matters because of #653. #653 asks for the leader's rejection to be shared with joiners. A smaller fix was available and rejected for this reason: resolving a sentinel instead of rejecting would remove the hazard outright and save bytes, but it deletes the rejection channel #653 would build on. This leaves it intact.

Why at construction rather than in fail(). Attaching where the promise is born makes safety structural — it does not depend on who happens to join, or when, or on the order in which fail and a late join interleave. The promise is never in an unobserved-rejection state for a single tick.

Why not "only reject when waiters > 0", the issue's other suggestion. It leaves the promise permanently pending, which trades a crash for a hang and is a worse trap for the next reader. And refs is not a reliable proxy for "someone is listening": it is decremented by aborts, so a run can have refs === 1 with a promise someone still holds. Rejecting unconditionally keeps fail()'s contract honest; the guard makes it harmless.

A note on the sentinel. claim.fail(new Error('cache: leader run failed')) never reaches a user — the follower catches it and ignores it. It is a control signal meaning "don't wait for me", not an error report. Marking that signal handled is not a loss of information.

What I deliberately did not change

LeaderClaim.promise is never read. The leader has settle/fail — the write end — and awaiting its own promise before settling would deadlock. Dropping the field is tempting and I implemented it, then reverted it: it is a public type change on the stitchapi/cache subpath (LeaderClaim is in lib/cache.d.ts's export list), and measured at 3 gzip bytes it pays for none of the budget below. That is a decision worth making on its own, not as a rider on a crash fix. The interface now carries a comment saying why the field must never be awaited.

Regression tests — three, and all three fail without the fix

Reverting packages/core/src/cache.ts to origin/main and leaving the tests in place:

× test/cache-internals.spec.ts > InflightCoalescer > a LONE leader failure is not an unhandled rejection (#670)
× test/cache.spec.ts > cache — in-process coalescing > a LONE failing leader leaves no unhandled rejection (#670)
× test/cache.spec.ts > cache — a handled failure does not kill the process (#670) > the issue's reproduction exits 0 and reaches the line after the call
   Tests  3 failed | 53 passed (56)

with the failures reading, respectively, expected [ Error: cache: leader run failed ] to deeply equal [] and expected 'handled: ok=false\n' to contain 'STILL ALIVE'. Restored: 56 passed (56).

Two in-process and one subprocess, because they prove different things:

1. cache-internals.spec.ts — the coalescer's own contract: a lone join + fail, no engine involved.
2. cache.spec.ts, engine level — mirrors the precedent at cache.spec.ts:339-361 (the LRU-eviction guard, which already established that fire-and-forget promises in the cache path must not surface). Written as a single sequential call, as the issue insists: a burst hides the bug, which is precisely why the existing "a leader failure is NOT shared" test a few lines above never caught it.
3. A subprocess test. The in-process assertions prove an unhandledRejection event does not fire. What a caller experiences is the exit code, and a test runner installs handlers of its own — so an in-process test can pass while the same code still kills a real program. This one bundles src/ with esbuild (already a devDependency, already how scripts/bundle-size.mjs measures), writes the issue's reproduction, and spawns a bare node with NODE_OPTIONS stripped so the default --unhandled-rejections=throw really is in force. It asserts exit 0 and that stdout reaches the line after the call. Bundling from src rather than lib means it needs no prior pnpm build and tests the working tree. Cost: ~140 ms.

Neighbours checked

• A coalesced follower still receives the leader's failure. 3 concurrent callers, failing vendor: ok=false/false/false, messages ["HTTP 503","HTTP 503","HTTP 503"], 3 adapter calls. Identical before and after. Failure is still not shared (each follower re-runs independently) — that is a coalesced failure is not shared, and a store silently un-pools pool: 'host' #653's subject, unchanged here.
• A successful coalesced run is unaffected. 3 concurrent callers, healthy vendor: 1 adapter call, all three served {"hit":1}. Identical before and after.
• The full suite: 139 files / 1491 tests, all green.

A second finding — reported, not fixed

The import { stitch } budget is charging for a chunk a consumer's bundler would split out. cache.ts's own header says it is "shipped behind the stitchapi/cache subpath so import { stitch } pulls none of it", and at runtime that is true — the engine reaches it by a lazy import('./cache') only when a stitch carries a cache block. But bundle-size.mjs measures with esbuild and no splitting, so the dynamic import is inlined and counted. Measured both ways on this branch:

So a real bundler keeps ~3.3 KB gzip of cache code out of the initial payload of import { stitch }, and the advertised figure overstates by roughly that. I have not touched the gate — changing its methodology would move the advertised number by ~3 kB, which is a far bigger maintainer-visible decision than this fix and does not belong on a crash fix. Flagging it so it can get its own issue.

Bundle budget — raised, and the advertised figure moves

Stated up front because the gate's own comment says a raise must be deliberate.

A minimum step, not the ~0.2 KB this gate usually restores — matching #477/#524/#485 and, most recently, #675: this is a fix squeezing past a full ceiling, not a new capability, and main had run down to one byte. Sizing a larger step is the maintainer's call, not a bug fix's; I will happily widen it if you prefer the #620 philosophy ("so the next small core-path fix is not gated on a budget PR of its own"), because at 41 B and 14 B the next one certainly will be.

There is no version of this that fits under a zero-byte ceiling. The guard is ~11 gzip bytes; the cheaper alternatives were measured and rejected above on correctness grounds, not size.

The advertised figure moves — and not because of this change. 21.5 KB is both the budget and a rounding boundary (22016 B), and main measured 22015 B: one byte below both. Any core-path byte takes import { stitch } from ~21 → ~22 kB. Nine figures across the six sites under the bundle-advertised-size tether — both READMEs, the installation and principles pages, the home-page metrics component, and the docs' source blurb — propagated by hand and then verified with the tether, not assumed:

ok  bundle-advertised-size  —  all sites agree on "22, 24"
6 tether(s): 6 ok, 0 fixed, 0 drift, 0 conflict, 0 broken

Two things worth knowing about that propagation:

• The core README's ~21 kB brotli moved with them and is more accurate for it: the whole entry's brotli is 21.6 KB, which rounds to 22, not 21. It was stale before.
• feat(core): QUERY is a read, not a write (Refs #462) #675 raises the same budget and moves the same nine figures, so these two will conflict on bundle-size.mjs and the six doc files. Whichever lands second should re-run the tether rather than trust the merge; the doc half is a pure value swap either way. yakir.lock is not touched here — its baseline was already behind on five tethers before this branch, and check passes on site agreement.

Verification

Every gate, run to completion:

Nothing failed. CHANGELOG entry under Unreleased → Fixed.

🤖 Generated with Claude Code